Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(dashboards): remove last_refresh from dashboard_tile #24004

Merged
merged 7 commits into from
Jul 26, 2024

Conversation

thmsobrmlr
Copy link
Contributor

@thmsobrmlr thmsobrmlr commented Jul 25, 2024

Problem

Going down a rabbit hole on what the updateTileOnDashboards that was added in #12271 does, I found that it's main purpose was updating the last_refresh attribute of a dashboard tile (not the attribute of the insight, but the tile!). It looks like we do not use this attribute anywhere anymore and the implementation was broken meanwhile, so that the original intention of updateTileOnDashboards isn't working any more. For example when you (1) open a dashboard (2) navigate to an insight (3) edit it's name (4) navigate back, the insight name would not have been updated.
2024-07-26 10 03 28

Changes

This PR removes unused functionality and fixes insight updates on dashboards like the above. It also refactors two usages of insight.dashboards (deprecated) to use insight.dashboard_tiles instead.
2024-07-26 09 54 50

How did you test this code?

CI run + testing the insight update

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Copy link
Contributor

github-actions bot commented Jul 25, 2024

Size Change: +31 B (0%)

Total Size: 1.07 MB

ℹ️ View Unchanged
Filename Size Change
frontend/dist/toolbar.js 1.07 MB +31 B (0%)

compressed-size-action

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@thmsobrmlr thmsobrmlr marked this pull request as ready for review July 26, 2024 08:04
@thmsobrmlr thmsobrmlr requested a review from a team July 26, 2024 08:27
@thmsobrmlr thmsobrmlr merged commit 59b6501 into master Jul 26, 2024
96 checks passed
@thmsobrmlr thmsobrmlr deleted the dashboard-tile-without-last-refresh branch July 26, 2024 09:55
silentninja pushed a commit to silentninja/posthog that referenced this pull request Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants